feat(tui): add session delete confirmation#444
Open
egdev6 wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an interactive “delete session” flow to the TUI sessions screen, including a confirmation prompt, keybindings, and coverage for the new behavior.
Changes:
- Render a session delete confirmation prompt in the Sessions view and update help text to advertise the delete keybinding.
- Add delete prompt state + delete command/message handling to the TUI update loop.
- Add unit tests for the prompt rendering and delete keyflow (confirm/cancel, success/failure).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/view.go | Renders delete confirmation prompt and updates Sessions help text to include delete key. |
| internal/tui/update.go | Handles delete prompt keyflow, processes delete results, and clamps cursor/scroll after sessions refresh. |
| internal/tui/model.go | Adds delete-related model state, message type, and delete command. |
| internal/tui/view_test.go | Tests that the delete prompt renders expected content. |
| internal/tui/update_test.go | Tests delete prompt open/cancel/confirm flows and error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Addressed the latest Copilot comments in commit
Validation after the fix:
PR still needs a maintainer to add exactly one label: |
Contributor
Author
|
@Alan-TheGentleman ready for review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked Issue
Closes #94
🏷️ PR Type
type:bug— Bug fixtype:feature— New featuretype:docs— Documentation onlytype:refactor— Code refactoring (no behavior change)type:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking change📝 Summary
dkeybinding on the TUI sessions list to delete the selected session.store.DeleteSession.📂 Changes
internal/tui/model.gointernal/tui/update.gointernal/tui/view.gointernal/tui/update_test.gointernal/tui/view_test.go🧪 Test Plan
go test ./...go test -tags e2e ./internal/server/...Additional focused validation:
go test ./internal/tui/...go test ./internal/store -run DeleteSessiongo test -cover ./internal/tui ./internal/store(internal/tui: 99.0%,internal/store: 78.3%)🤖 Automated Checks
These run automatically and all must pass before merge:
Closes #N/Fixes #N/Resolves #Nstatus:approvedlabeltype:*labelgo test ./...passesgo test -tags e2e ./internal/server/...passes✅ Contributor Checklist
Closes #N)type:*label to this PRgo test ./...go test -tags e2e ./internal/server/...Co-Authored-Bytrailers in commits💬 Notes for Reviewers
This implements only the remaining TUI scope from the latest maintainer comment on #94. Store, HTTP, and CLI session deletion already exist; this PR reuses
store.DeleteSessionand lets the store enforce safety rules such as blocking sessions with observations.